Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adapt to eip-1193 provider changes #317

Merged
merged 17 commits into from
Jul 23, 2024

Conversation

cryptodev-2s
Copy link
Contributor

After SafeEventEmitterProvider is updated to support EIP-1193 and a new version of @metamask/eth-json-rpc-provider is released, we should adapt to the changes:

  • We should bump @metamask/eth-json-rpc-provider to rely on the new changes.
  • At that point, calling sendAsync will be deprecated; we should use request instead.
    • There are two middleware which use sendAsync: retryOnEmpty and block-ref.
    • We also use sendAsync in providerAsMiddleware.
    • Finally, there are a lot of tests that mock sendAsync on the provider. The tests use a helper, stubProviderRequests which does this in particular. These references need to be changed.

Copy link

socket-security bot commented Jul 16, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/[email protected] None 0 63.6 kB mcmire
npm/@metamask/[email protected] None 0 86.7 kB metamaskbot

🚮 Removed packages: npm/@metamask/[email protected], npm/@metamask/[email protected]

View full report↗︎

yarn.lock Show resolved Hide resolved
Copy link
Contributor

@legobeat legobeat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/retryOnEmpty.ts Outdated Show resolved Hide resolved
src/retryOnEmpty.ts Outdated Show resolved Hide resolved
Copy link

socket-security bot commented Jul 18, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: npm/@metamask/[email protected]

View full report↗︎

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@cryptodev-2s cryptodev-2s requested a review from legobeat July 18, 2024 16:24
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall approach here makes sense to me. Most of my comments below are minor.

src/block-ref.ts Outdated Show resolved Hide resolved
src/block-ref.ts Show resolved Hide resolved
src/retryOnEmpty.test.ts Outdated Show resolved Hide resolved
src/retryOnEmpty.ts Outdated Show resolved Hide resolved
src/providerAsMiddleware.ts Outdated Show resolved Hide resolved
src/retryOnEmpty.ts Outdated Show resolved Hide resolved
test/util/helpers.ts Outdated Show resolved Hide resolved
test/util/helpers.ts Outdated Show resolved Hide resolved
src/retryOnEmpty.test.ts Outdated Show resolved Hide resolved
src/block-ref.test.ts Outdated Show resolved Hide resolved
@mcmire
Copy link
Contributor

mcmire commented Jul 19, 2024

@SocketSecurity ignore npm/@metamask/[email protected]

This is our own package.

@cryptodev-2s cryptodev-2s requested a review from mcmire July 22, 2024 16:09
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had some suggestions, but overall, the approach makes sense.

src/providerAsMiddleware.ts Show resolved Hide resolved
src/block-ref.ts Outdated Show resolved Hide resolved
src/retryOnEmpty.ts Outdated Show resolved Hide resolved
@cryptodev-2s
Copy link
Contributor Author

cryptodev-2s commented Jul 22, 2024

@SocketSecurity ignore npm/@metamask/[email protected]

This is our own package.

Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@cryptodev-2s cryptodev-2s requested review from legobeat and removed request for legobeat July 22, 2024 19:52
@cryptodev-2s cryptodev-2s merged commit c193b59 into main Jul 23, 2024
19 checks passed
@cryptodev-2s cryptodev-2s deleted the feat/adapt-eip-1193-provider-changes branch July 23, 2024 08:50
@legobeat legobeat mentioned this pull request Jul 23, 2024
MajorLift added a commit that referenced this pull request Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adapt to EIP-1193 provider changes
3 participants